Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(server): system config #9517

Merged
merged 1 commit into from
May 15, 2024
Merged

refactor(server): system config #9517

merged 1 commit into from
May 15, 2024

Conversation

jrasm91
Copy link
Contributor

@jrasm91 jrasm91 commented May 15, 2024

  • Move server config implementation to just store a single diff in system-metadata instead of a dedicated table of single key/value overrides
  • Make it much easier to add new configuration properties in the future

@jrasm91 jrasm91 requested a review from zackpollard May 15, 2024 16:10
@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: 22e0c47
Status: ✅  Deploy successful!
Preview URL: https://118d7479.immich.pages.dev
Branch Preview URL: https://refactor-server-config.immich.pages.dev

View logs

jrasm91
jrasm91 commented May 15, 2024
View reviewed changes
Copy link
Contributor Author

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has a lot of noise, but the only meaningful changes are in the system-config.core.ts file and the migration.

server/src/cores/system-config.core.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file

server/src/migrations/1715787369686-RemoveSystemConfigTable.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file

@jrasm91 jrasm91 marked this pull request as ready for review May 15, 2024 16:20
@jrasm91 jrasm91 requested a review from danieldietzler as a code owner May 15, 2024 16:20
zackpollard
zackpollard approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@zackpollard zackpollard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested this but the changes look good, if you want to merge it go ahead, I will give it an actual test tomorrow regardless 😄

@jrasm91 jrasm91 merged commit 984aa8f into main May 15, 2024
23 of 24 checks passed
@jrasm91 jrasm91 deleted the refactor/server-config branch May 15, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danieldietzler danieldietzler danieldietzler approved these changes

@zackpollard zackpollard zackpollard approved these changes

Assignees
No one assigned
Labels
🗄️server
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jrasm91 @zackpollard @danieldietzler